Fix axis Bug in MKLDNN Softmax #11335
Conversation
src/operator/nn/softmax.cc
Outdated
| @@ -38,10 +38,9 @@ static void SoftmaxComputeExCPU(const nnvm::NodeAttrs& attrs, | |||
| const std::vector<NDArray>& inputs, | |||
| const std::vector<OpReqType>& req, | |||
| const std::vector<NDArray>& outputs) { | |||
| const SoftmaxParam& param = nnvm::get<SoftmaxParam>(attrs.parsed); | |||
| // It seems MKLDNN softmax doesn't support training. | |||
| // and it only supports non-negative axis. | |||
There was a problem hiding this comment.
why mkldnn softmax doesn't support training? https://github.com/intel/mkl-dnn/blob/master/include/mkldnn.hpp#L2680
There was a problem hiding this comment.
We are not using the latest MKLDNN.
|
@szha unit tests with negative axis have been added and passed. |
|
@szha what's the difference of softmax/log_softmax/SoftmaxActivation in mxnet? and which one is the most widely used by mxnet users? Seems only softmax is optimized here with mkldnn primitive. |
|
right, the SoftmaxActivation is an older operator and is not actively developed anymore. Going forward we are using softmax/log_softmax. |
|
@szha same question for SoftmaxOutput operator? https://github.com/apache/incubator-mxnet/blob/master/src/operator/softmax_output.cc#L47 |
* add softmax imporvement * reuse CheckAxis code * update comment * add tests with negative axis
* add softmax imporvement * reuse CheckAxis code * update comment * add tests with negative axis
Description
Apply CheckAxis to MKLDNN Softmax function in order to call MKLDNN correctly.
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
@pengzhao-intel @TaoLv